-
Notifications
You must be signed in to change notification settings - Fork 55
fix: improve notification close button hover detection #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The close button in notifications was difficult to click because the hover detection area was too small. Changed the FocusScope to a Control component with hover tracking capability. Added hover state as a trigger condition for displaying the close button, making it easier for users to close notifications by expanding the interactive area. Log: Improved notification close button interaction by expanding hover detection area Influence: 1. Test hovering over notification area to verify close button appears 2. Verify close button functionality works correctly when clicked 3. Test with notifications containing actions to ensure strongInteractive mode still works 4. Check close button visibility in different notification states 5. Verify accessibility and keyboard navigation still functions properly fix: 改进通知关闭按钮悬停检测 通知中的关闭按钮难以点击,因为悬停检测区域太小。将 FocusScope 改为具有悬 停跟踪功能的 Control 组件。添加悬停状态作为显示关闭按钮的触发条件,通过 扩大交互区域使用户更容易关闭通知。 Log: 通过扩大悬停检测区域改进了通知关闭按钮的交互体验 Influence: 1. 测试悬停在通知区域时关闭按钮是否正常显示 2. 验证点击关闭按钮功能是否正常工作 3. 测试包含操作的通知,确保强交互模式仍然有效 4. 检查不同通知状态下关闭按钮的可见性 5. 验证辅助功能和键盘导航是否仍然正常工作
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnhanced notification close button interaction by expanding the hoverable area and switching to a hover-aware placeholder component, ensuring the close button appears more reliably when hovered. Class diagram for updated NotifyItemContent structureclassDiagram
class NotifyItem {
bool closeVisible
int miniContentHeight
bool enableDismissed
}
class Control {
bool hovered
}
class SettingActionButton {
string objectName
}
NotifyItem o-- Control : uses
Control o-- SettingActionButton : loads
NotifyItem o-- SettingActionButton : closeBtn
NotifyItem : closeVisible = activeFocus || impl.hovered
Control : hovered
SettingActionButton : objectName = "closeNotify-" + root.appName
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Enable hoverEnabled: true on the new Control placeholder so that closePlaceHolder.hovered actually receives hover events.
- Simplify the Loader’s active binding by removing the redundant activeFocus check since closeVisible already includes focus state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Enable hoverEnabled: true on the new Control placeholder so that closePlaceHolder.hovered actually receives hover events.
- Simplify the Loader’s active binding by removing the redundant activeFocus check since closeVisible already includes focus state.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, mhduiy, robertkill The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
2 similar comments
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, mhduiy, robertkill The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, mhduiy, robertkill The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这段代码是一个QML文件,用于通知面板中的通知项内容。我来分析一下代码的改进: 语法逻辑
代码质量
代码性能
代码安全
总体评价这些改进主要是为了提高用户体验和代码质量,通过更合理的条件判断和组件选择,使得通知项的交互更加直观和友好。代码结构也更加清晰,便于后续的维护和扩展。建议继续保持这种良好的代码风格,并在后续开发中继续关注用户体验和代码质量。 |
The close button in notifications was difficult to click because the
hover detection area was too small. Changed the FocusScope to a Control
component with hover tracking capability. Added hover state as a trigger
condition for displaying the close button, making it easier for users to
close notifications by expanding the interactive area.
Log: Improved notification close button interaction by expanding hover
detection area
Influence:
strongInteractive mode still works
fix: 改进通知关闭按钮悬停检测
通知中的关闭按钮难以点击,因为悬停检测区域太小。将 FocusScope 改为具有悬
停跟踪功能的 Control 组件。添加悬停状态作为显示关闭按钮的触发条件,通过
扩大交互区域使用户更容易关闭通知。
Log: 通过扩大悬停检测区域改进了通知关闭按钮的交互体验
Influence:
Summary by Sourcery
Improve hover detection area for notification close button by replacing FocusScope with Control and updating visibility logic to show the button on hover.
Bug Fixes:
Enhancements: